Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

[Java] Reformated Design Strategies to Use Codeblocks #2308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

[Java] Reformated Design Strategies to Use Codeblocks #2308

wants to merge 5 commits into from

Conversation

shbenzer
Copy link
Contributor

@shbenzer shbenzer commented May 9, 2025 •
edited by qodo-merge-pro bot
Loading

User description

Updated Design Strategies to use codeblock design for java examples and future

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

PR Type

Enhancement, Documentation

Description

  • Refactored Java code examples in design strategies documentation to use code blocks

    • Replaced inline Java snippets with gh-codeblock references for maintainability
    • Ensured consistency across English, Japanese, Portuguese, and Chinese docs

  • Added comprehensive Java example file BestPractices.java for code block inclusion

    • Contains multiple Page Object and utility class examples for design strategies

  • Improved code snippet maintainability and translation readiness in documentation

Changes walkthrough 📝

Relevant files
Enhancement
BestPractices.java
Add comprehensive Java design strategies example file       

examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java

  • Added new Java file with multiple Page Object and utility class
    examples
  • Includes EditIssue, IssueList, EditIssueBetter, ProjectPage,
    SecuredPage, ActionBot, and test class
  • Designed for inclusion in documentation via code block references
    		• +284/-0 
    Documentation
    design_strategies.en.md
    Refactor Java code examples to use code blocks in English doc

    website_and_docs/content/documentation/test_practices/design_strategies.en.md

  • Replaced inline Java code snippets with gh-codeblock references to new
    Java file
  • Updated all relevant code tabs to use centralized code blocks
  • Improved maintainability and consistency of Java examples in
    documentation
  • Minor update to Python code block reference for completeness
    		• +199/-319
    design_strategies.ja.md
    Refactor Java code examples to use code blocks in Japanese doc

    website_and_docs/content/documentation/test_practices/design_strategies.ja.md

  • Replaced inline Java code snippets with gh-codeblock references to new
    Java file
  • Updated all Java code tabs for consistency and maintainability
  • Ensured translation structure remains intact
  • Minor update to Python code block reference for completeness
    		• +199/-318
    design_strategies.pt-br.md
    Refactor Java code examples to use code blocks in Portuguese doc

    website_and_docs/content/documentation/test_practices/design_strategies.pt-br.md

  • Replaced inline Java code snippets with gh-codeblock references to new
    Java file
  • Updated all Java code tabs for consistency and maintainability
  • Ensured translation structure remains intact
  • Minor update to Python code block reference for completeness
    		• +199/-318
    design_strategies.zh-cn.md
    Refactor Java code examples to use code blocks in Chinese doc

    website_and_docs/content/documentation/test_practices/design_strategies.zh-cn.md

  • Replaced inline Java code snippets with gh-codeblock references to new
    Java file
  • Updated all Java code tabs for consistency and maintainability
  • Ensured translation structure remains intact
  • Minor update to Python code block reference for completeness
    		• +199/-318
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @shbenzer shbenzer self-assigned this May 9, 2025
    @netlify Netlify
    Copy link

    netlify bot commented May 9, 2025 •
    edited
    Loading

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 8a597e3

    @qodo-merge-pro Qodo Merge Pro
    Copy link
    Contributor

    qodo-merge-pro bot commented May 9, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Method Parameter Error

    In the setSeleniumVersion method, line 48 uses logOutput variable instead of seleniumVersion parameter, which would cause incorrect data to be entered in the form.

     
    public void setSeleniumVersion(String seleniumVersion) {
  WebElement field = driver.findElement(By.id("issue_form_selenium-version"));
  clearAndType(field, seleniumVersion);
}
    Incomplete Implementation

    The IssueList class extends LoadableComponent but doesn't implement the required load() and isLoaded() methods, which would cause runtime errors.

     
    public class IssueList extends LoadableComponent<IssueList> {
    private final WebDriver driver;

    public IssueList(WebDriver driver) {
        this.driver = driver;
    }

}

    @qodo-merge-pro Qodo Merge Pro
    Copy link
    Contributor

    qodo-merge-pro bot commented May 9, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    ✅ Implement required abstract methods
    Suggestion Impact:The suggestion was implemented by adding the required abstract methods load() and isLoaded() to the IssueList class. The class was moved to the end of the file and the methods were implemented, though with incorrect return types.

    code diff:

    +class IssueList extends LoadableComponent<IssueList> {
+  private final WebDriver driver;
+
+  public IssueList(WebDriver driver) {
+      this.driver = driver;
+  }
+
+  @Override
+  protected void load() {
+    return true;
+  }
+
+  @Override
+  protected void isLoaded() throws Error {
+    return true;
+  }
+
+}

    The IssueList class extends LoadableComponent but doesn't implement the required
    abstract methods load() and isLoaded(). These methods are necessary for the
    LoadableComponent pattern to work correctly.

    examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java [77-84]

     public class IssueList extends LoadableComponent<IssueList> {
     private final WebDriver driver;
 
     public IssueList(WebDriver driver) {
         this.driver = driver;
     }
-
+    
+    @Override
+    protected void load() {
+        // Implementation needed
+    }
+    
+    @Override
+    protected void isLoaded() throws Error {
+        // Implementation needed
+    }
 }

    [Suggestion processed]

    Suggestion importance[1-10]: 10

    __

    Why: The IssueList class extends LoadableComponent but doesn't implement the required abstract methods load() and isLoaded(), which would cause compilation errors. These methods are mandatory when extending the LoadableComponent class.

    		High
    ✅ Fix generic type parameter
    Suggestion Impact:The commit changed the generic type parameter in EditIssueBetter class from EditIssue to EditIssueBetter, exactly as suggested

    code diff:

    -public class EditIssueBetter extends LoadableComponent<EditIssue> {
+// class IssueList extends LoadableComponent<IssueList> {
+//     private final WebDriver driver;
+
+//     public IssueList(WebDriver driver) {
+//         this.driver = driver;
+//     }
+
+// }
+
+class EditIssueBetter extends LoadableComponent<EditIssueBetter> {

    The generic type parameter in LoadableComponent is incorrect. It should be
    EditIssueBetter since this class is extending LoadableComponent for itself, not
    for the EditIssue class.

    examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java [86]

    -public class EditIssueBetter extends LoadableComponent<EditIssue> {
+public class EditIssueBetter extends LoadableComponent<EditIssueBetter> {

    [Suggestion processed]

    Suggestion importance[1-10]: 9

    __

    Why: The generic type parameter is incorrect and would cause compilation issues. The class should extend LoadableComponent<EditIssueBetter> since it's implementing the LoadableComponent pattern for itself, not for the EditIssue class.

    		High
    General
    Improve element existence check

    The code finds an element but doesn't use it, which is inefficient. Since you're
    only checking for existence, use findElements() and check the size instead of
    catching an exception, which is more performant.

    examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java [222-226]

    -try {
-  WebElement div = driver.findElement(By.id("multilogin-dropdown"));
-} catch (NoSuchElementException e) {
+if (driver.findElements(By.id("multilogin-dropdown")).isEmpty()) {
   Assertions.fail("Cannot locate user name link");
 }
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: Using findElements() and checking if the list is empty is more efficient than catching a NoSuchElementException. This approach avoids the overhead of exception handling and follows best practices for element existence checks in Selenium.

    		Medium
    • Update

    @shbenzer
    Copy link
    Contributor Author

    shbenzer commented May 9, 2025

    Better implementation of #2301

    @shbenzer
    Copy link
    Contributor Author

    Failures are unrelated to this pr

    @shbenzer shbenzer requested a review from harsha509 May 11, 2025 22:11
    harsha509
    harsha509 reviewed May 30, 2025
    View reviewed changes
    Copy link
    Member

    @harsha509 harsha509 left a comment •
    edited
    Loading

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Hi @shbenzer ,

    Thank you for the PR. can you look into below comments

    Thanks,
    Sri

    examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java
    }
    }

    // class IssueList extends LoadableComponent<IssueList> {
    Copy link
    Member

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    can we remove commented code (or) is this intensional ?

    Copy link
    Contributor Author

    @shbenzer shbenzer May 30, 2025 •
    edited
    Loading

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This is intentional so that there aren't failures, but the code is referenced in the docs. They were in their own codeblocks in docs and I pulled them out so we could better switch b/w languages for examples

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    I can look into tweaking the docs a bit so these commented sections aren't necessary sometime soon

    examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java
    }
    }

    // public class FooTest {
    Copy link
    Member

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    same as above !

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @harsha509 harsha509 harsha509 left review comments

    Assignees

    @shbenzer shbenzer

    Labels
    Review effort 2/5
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    2 participants
    @shbenzer @harsha509